-
Notifications
You must be signed in to change notification settings - Fork 256
Append the label value of the primary spans of diagnostics to the LSP diagnostics message if it is available. #447
Append the label value of the primary spans of diagnostics to the LSP diagnostics message if it is available. #447
Conversation
This is great! I think we regressed at some point, so that's great to see it back. I know I found myself running Looking at the However, I'm not sure if appending label in case of every error is desired. The message |
@Xanewok , thanks for the comment. This is indeed a bit heavy handed. I was kind of curious what would have gotten caught by that. I did this in the dead of night, frustrated at rust-lang/vscode-rust#103. Hmm, not sure why that test passed on my Mac. It seemed really weird for it to pass and I usually intentionally break areas to understand what tests touch that area. Nothing broke, so I just plowed forward. It's a bit embarrassing for me to put a PR with a broken test out the gate. Anyway, I'll take a closer look. |
Ack, I was a bit too tired to realize I didn't actually run the test after the changes. That sequence of events is important. |
… diagnostics message if it is available. This is very useful in diagnostic messages where the label has specific diagnostics information for the site and little in the message. The most popular message and an example is detailed as follows: In cases like E0308 or better known as "mismatched types", the message is simply "mismatched types". Users seeing "mismatched types" in their editing buffers would have had to immediately step out of their buffers and run whatever compiles the line in their terminals to take a look at what is described here as the label in order to figure out what and how the types are in conflict. Related: rust-lang/vscode-rust#103
…pans The message is a bit useless at the moment but LSP does not have a good way to do multi-span and it might be best to leave this mildly useless message until they do so instead of special-casing it to not appear. Related: microsoft/vscode#1927
628e248
to
304b97b
Compare
I fixed the test. I'm not sure what to do about the story where there are two spans. Should I modify this to only append the label if there's just a singular span and punt the two span issue until the LSP/VSCode issue below is resolved? Or is this minimal logic good enough to get RLS more immediately useful for the moment? As for properly displaying two spans, the multi-span story is a bit not developed at the moment. It doesn't appear LSP has a way to display that at the moment. Here's the related issue in VSCode: |
Test for if the compiler message *AND* how the mismatched types failed expectations are included in LSP's diagnostic message.
This might belong in another issue but a VSCode/LSP developer took a look at this situation nearly a year ago and suggested it might be just best to add more strings of messages like this to the LSP Diagnostic message with regards to the multiple span issue. IMO I just wanted the primary span's label for now for the mismatched types issue. It would be a bit of scope creep to try to add more than that. |
Hey, thanks for the PR! So, we used to do this and I deliberately changed to not doing it because there seemed to be more cases where it gave bad information and it meant that the errors in the 'problems' panel were a pain to read. Unfortunately there seems to be no good way to fix this. My plan for a 'least worst' solution to this is to record the extra information for each error (and the affected span) in the RLS and then return the info as part of the |
Ah yeah, this is indeed too broad. Still, it would be nice if E0308 gave a nice message like Typescript's. |
This is very useful in diagnostic messages where the label has specific
diagnostics information for the site and little in the message. The most
popular message and an example is detailed as follows:
In cases like E0308 or better known as "mismatched types", the message
is simply "mismatched types".
Users seeing "mismatched types" in their editing buffers would have had
to immediately step out of their buffers and run whatever compiles the
line in their terminals to take a look at what is described here as the
label in order to figure out what and how the types are in conflict.
Related: rust-lang/vscode-rust#103